Skip to content

Android APK: make FixRuntimePackAssetTypes actually strip desktop-RID native .so files - #225

Merged
winnerspiros merged 1 commit into
masterfrom
copilot/check-last-build-failure
Apr 21, 2026
Merged

Android APK: make FixRuntimePackAssetTypes actually strip desktop-RID native .so files#225
winnerspiros merged 1 commit into
masterfrom
copilot/check-last-build-failure

Conversation

Copilot AI commented Apr 21, 2026

Copy link
Copy Markdown

Build v2026.421.144 shipped an APK with the Linux-glibc libbass.so from ppy.osu.Framework.NativeLibs runtimes/linux-arm64/native/ packed into lib/arm64-v8a/ instead of the Android ELF from ppy.osu.Framework.Android's AAR. Bionic cannot load glibc-versioned symbols, so the app would crash at startup with System.DllNotFoundException: bass; the architecture-sanity verify step correctly failed the build.

Root cause

The FixRuntimePackAssetTypes target in osu.Android.props (PR #224) was a silent no-op:

  • Single-backslash literal '\' inside an MSBuild property function is ambiguous to the expression parser — the canonical form is '\\'.
  • Conditional Remove="@(X)" with an inline nested OR of metadata-batched Contains calls is fragile.

Changes

  • Two-step helper-list + Remove-by-identity idiom instead of conditional Remove. Batching is per-item on the Include's Condition; the subsequent Remove is an unconditional identity match.
  • Fixed '\\' escape in all path normalisation calls.
  • Single inverted match replaces the enumerated RID OR chain — strip any .so whose path contains /runtimes/ but not /runtimes/android. Covers linux-*, osx-*, ios-*, maccatalyst-*, win-*, browser-*, freebsd-*, … in one rule and auto-covers future RIDs.
  • Applied to ReferenceCopyLocalPaths and RuntimeCopyLocalItems in addition to ResolvedFileToPublish, since NuGet native assets can flow through any of the three.
  • AssetType=native marking on the surviving Android items is preserved so they still land in lib/<abi>/.
<_DesktopNativeToStripFromPublish Include="@(ResolvedFileToPublish)"
  Condition="'%(Extension)' == '.so'
             AND $([System.String]::Copy('%(Identity)').Replace('\\','/').Contains('/runtimes/'))
             AND !$([System.String]::Copy('%(Identity)').Replace('\\','/').Contains('/runtimes/android'))" />
<ResolvedFileToPublish Remove="@(_DesktopNativeToStripFromPublish)" />

Considered and rejected: workflow-level "strip & re-sign" safety net

The broken APK contains only the Linux libbass.so (same path collided and overwrote the AAR one during packaging). Stripping it in post would leave the APK with no libbass.so at all, which the existing verify step would then fail as "missing". A real rescue would have to unpack ppy.osu.Framework.Android's AAR from the NuGet cache and re-inject the correct ELF — brittle for no gain, since the ground-truth verify step already fails the build cleanly before anything is uploaded.

Dependency integrity

No package source or version changes. All ppy.osu.Framework* references still resolve against the winnerspiros GitHub Packages feed, and the vendored ppy.Veldrid.SPIRV winnerspiros fork pin in local-packages/ is untouched.

@winnerspiros
winnerspiros marked this pull request as ready for review April 21, 2026 17:25
Copilot AI review requested due to automatic review settings April 21, 2026 17:25
@winnerspiros
winnerspiros merged commit 45b12a5 into master Apr 21, 2026
16 of 20 checks passed
@gitar-bot

gitar-bot Bot commented Apr 21, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Android build MSBuild props to ensure desktop-RID native .so files (e.g., Linux glibc libbass.so) are stripped from the APK publish inputs so only Android-compatible natives are packaged into lib/<abi>/.

Changes:

  • Reworks FixRuntimePackAssetTypes to use helper item lists + unconditional identity Remove, avoiding fragile conditional Remove.
  • Normalizes path separators using the correct '\\' escape in MSBuild property functions.
  • Applies stripping to ResolvedFileToPublish, ReferenceCopyLocalPaths, and RuntimeCopyLocalItems while preserving AssetType=native marking for Android runtime assets.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread osu.Android.props
Comment on lines +103 to +105
* Uses a single inverted match: any `.so` under `/runtimes/<rid>/native/` whose
RID does not start with `android` is stripped. This covers every non-Android
desktop/mobile RID in one rule and is future-proof against new RIDs.

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation note says the rule targets .so files under /runtimes/<rid>/native/, but the actual filter only checks for Contains('/runtimes/') (no /native/ component). Either update the note to match the broader behavior, or tighten the filter to include the /native/ segment if that’s the intended scope, to avoid accidentally stripping any future .so that might live under a different runtimes/<rid>/... subfolder.

Suggested change
* Uses a single inverted match: any `.so` under `/runtimes/<rid>/native/` whose
RID does not start with `android` is stripped. This covers every non-Android
desktop/mobile RID in one rule and is future-proof against new RIDs.
* Uses a single inverted match: any `.so` under `/runtimes/` whose runtime-specific
path does not correspond to an `android` RID is stripped. This intentionally covers
every non-Android desktop/mobile RID in one rule and is future-proof against new RIDs.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants